Skip to content

Clarify that calculateBoundingBox is aligned to the axes. #719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Invariance-NaN
Copy link

The current docs for calculateBoundingBox are not quite correct, since the "smallest rectangular prism" containing the geometry need not be aligned to the axes.

@ksen0
Copy link
Member

ksen0 commented Aug 11, 2025

Thanks for raising this @Invariance-NaN !

The reference is automatically built from the p5.js core source code. To change this part of the reference, this file should be changed: https://github.com/processing/p5.js/blob/db7747174b32ab1851547f0212e5b8e24d2064cb/src/webgl/p5.Geometry.js#L683 As suggested in the contributor guidelines, in the future / in general, please make an issue first, and explain your proposal there, before working on a solution - so that the solution can first be discussed.

I'll close this PR lease feel free to open an issue/PR in that repository!

In terms of the suggestion itself, I think pointing out that the outcome needs, e.g., translation (in the example translate(bbox.offset.x, bbox.offset.y, bbox.offset.z);) would be helpful, however I think the first line of the reference should remain as beginner-friendly as possible. To strike a balance, I'd suggest adding a 1 or 2 sentence paragraph, after "size (length) and offset (center).", that uses the axis-aligned bounding box terminology, and also explains it. I think it's important that if we use any technical terms, we also explain them in the reference, however I might be wrong in my understanding of the intent here, or maybe others have a better idea.

If you do make an issue/PR in the p5.js repository to make this suggestion, or an updated version, please feel free to tag me or @perminder-17 and one of us will be happy to review!

@ksen0 ksen0 closed this Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants